-
Notifications
You must be signed in to change notification settings - Fork 933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Storage: Show correct disk size #14595
base: main
Are you sure you want to change the base?
Conversation
5ad4a57
to
81fe9da
Compare
2a2e63c
to
7c08121
Compare
7c08121
to
4b13a2a
Compare
Is this only for container filesystem based volumes or for VM block volumes too? Is this key used for VM image volumes too? |
This is for all instance types, since all can be restrained by setting the device's
Yes, it is currently only used for image volumes. |
thats not what i asked, is it currently populated for VM image volumes as well as container image volumes? |
Oh sorry, I misread your question. Yes, it is used for both container and VM images. |
@hamistao as discussed we will revert/continue to return |
@hamistao there's a comment in
Does this need to be updated/considered/changed? |
@hamistao please can you explore whether we can prevent volatile.rootfs.size being editable for volumes that have it set, as a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make the changes previously discussed.
4b13a2a
to
a7085f0
Compare
ebd1bee
to
8084432
Compare
We have reached a crossroads when it comes to what value to use to represent the size of unbounded (sizeless) volumes. Previously, we got size 0 in every scenario except when a user explicitely set the volume size, either through Mind that these 0's were never exposed though CLI command outputs, so Since #14511, we now show -1 for the total size of sizeless instances (not custom volumes yet) and -1 for usage when it is not supported. We also now show the default block size for block-based instances (10GiB). Custom volumes still behave the same as they did previously. So now we have two possible ways to think of the size of sizeless volumes. One one hand, option A implies we should be "changing less" and also takes into account the possibility of some users relying on this 0 to check wether the instance has a root disk size set or not. It is also more flexible in case we want to make it a uunsigned integet in the future. I hope this was clear, if any info is missing we can discuss it in our 1-1 later, cc @tomponline |
@hamistao as discussed in our 1:1 today, we tested the current state APIs for disk usage for instances and custom volumes as follows: Instance state:
Custom volume state:
So it looks like we do return So we agreed to maintain the existing premise that unbounded volumes should return a size of 0. We can use -1 for total size in an error scenario if needed. We see this pattern already for CPU usage and processes fields. |
…limits" As per the discussion outlined on canonical#14595 (comment), we are sticking with using 0 to represent unbounded volumes. This reverts commit 26b8d73. Signed-off-by: hamistao <[email protected]>
…limits" As per the discussion outlined on canonical#14595 (comment), we are sticking with using 0 to represent unbounded volumes. This reverts commit 26b8d73. Signed-off-by: hamistao <[email protected]>
…limits" As per the discussion outlined on canonical#14595 (comment), we are sticking with using 0 to represent unbounded volumes. This reverts commit 26b8d73. Signed-off-by: hamistao <[email protected]>
…limits" As per the discussion outlined on canonical#14595 (comment), we are sticking with using 0 to represent unbounded volumes. This reverts commit 26b8d73. Signed-off-by: hamistao <[email protected]>
…limits" As per the discussion outlined on canonical#14595 (comment), we are sticking with using 0 to represent unbounded volumes. This reverts commit 26b8d73. Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
All the handling of `volatile.rootfs.size` for images considers that it can be a '123 GiB' type string as well, so this also considers these strings when validating the key Signed-off-by: hamistao <[email protected]>
… & delete effective fields Signed-off-by: hamistao <[email protected]>
…eateInstance` Signed-off-by: hamistao <[email protected]>
…eateInstanceFromConversion` Signed-off-by: hamistao <[email protected]>
…eateInstanceFromMigration` Signed-off-by: hamistao <[email protected]>
…eateInstanceFromImage` Signed-off-by: hamistao <[email protected]>
…eateInstanceFromCopy` Signed-off-by: hamistao <[email protected]>
…eateInstanceFromBackup` Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
This makes it so that the an instance volume's `volatile.rootfs.size` config is aligned with the volume's actual size in any point in time. i.e. some configs like disk device `size` and pool's `volume.size` can propagate into the volume's `volatile.rootfs.size`, similarly to what happens with custom volumes. Signed-off-by: hamistao <[email protected]>
For instance volumes that contain `volatile.rootfs.size`, derive disk size from this config key. For instances that precede the usage of thi config key on instances, use the old method as to avoid breaking the API when getting the size of those instances. Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
6a89ca3
to
4f1e6b0
Compare
This is a follow up PR for #14511 and this aims to address the problems outlined in #14511 (comment).
To do this, we need a way to determine wether the instance volume had its quota set based on
volume.size
on the pool level. For this, we are introducing thevolatile.rootfs.size
config key for instance volumes, previously it was only recognized in image volumes. This config key is used to represent the current quota of an instance volume at any point in time. This way we can determine whether a volume had its quota set based onvolume.size
or not.Also, any changes to the disk only cascade onto the volume's
volatile.rootfs.size
when the quota is applied on the volume. Thus, we are able to handle instances that don't support online resizing as the field we are looking for will only change when a quota is applied on the volume, i.e. when the instance restarts.This key was chosen instead of the
size
key because the intended behavior of this config key fits better the pattern we see withvolatile.*
keys, that can be changed by LXD itself, without the user specifying them.